feat(message)_: add PinnedBy to common.Message #6226
Conversation
jrainville
requested review from
ilmotta,
Cuteivist and
igor-sirotin
and removed request for
ilmotta and
Cuteivist
Jenkins BuildsClick to see older builds (8)
|
jrainville
force-pushed
the
feat/add-pinned-info-in-messages
branch
from
300545f to
f048766
Compare
jrainville
changed the title
Yes, that already worked before. I posted a video here: status-im/status-desktop#16897 (comment) |
There was a problem hiding this comment.
This saves us from having to keep a cache of the pinned messages and comparing all new messages.
@jrainville In status-mobile we keep the pinned messages state in sync with status-go via signal pinMessages. Pinned messages in mobile are indexed and stored in memory by chat ID & message ID, which makes checking if any message is pinned cheap.
The currently pinned messages per chat will continue to be stored in memory even with the changes from this PR (in mobile).
Maybe on desktop things are different? Could you elaborate a bit more? Maybe then I'll understand what/if something needs to change or be optimized on the mobile side.
@ilmotta We used to only have the pinned messages in a model made to be accessed in the PinnedMessagesPopup. This popup worked flawlessly. When the user clicked the button to see the pinned messages, they all appeared correctly. We didn't store them in a "cache" though, so when we scrolled up in a channel, the old messages didn't appear as pinned I did implement a cache solution in the chat module that would keep the pinned messages like you do and then apply the pinned status to new messages by looping them when they arrived. However, we found that if we did it using the way I just did here in status-go, it removes a lot of logic needed in the middle layer (Nim). So like you said, you can keep your current method and it will work perfectly. We just prefer to write as little Nim code as possible, so this solution solves it 😄 |
Needed for status-im/status-desktop#16896 Having `PinnedBy` directly in the Message object makes it way simpler in the client to know if a message is pinned. This saves us from having to keep a cache of the pinned messages and comparing all new messages.
jrainville
force-pushed
the
feat/add-pinned-info-in-messages
branch
from
f048766 to
98176d9
Compare
Needed for status-im/status-desktop#16896
Having
PinnedBydirectly in the Message object makes it way simpler in the client to know if a message is pinned. This saves us from having to keep a cache of the pinned messages and comparing all new messages.